refactor: FunctionSignature return type container#8717
refactor: FunctionSignature return type container#8717canerakdas wants to merge 3 commits intonodejs:mainfrom
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
👋 Codeowner Review RequestThe following codeowners have been identified for the changed files: Team reviewers: @nodejs/nodejs-website Please review the changes when you have a chance. Thank you! 🙏 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #8717 +/- ##
==========================================
- Coverage 75.10% 75.07% -0.04%
==========================================
Files 104 104
Lines 9167 9167
Branches 315 316 +1
==========================================
- Hits 6885 6882 -3
- Misses 2280 2283 +3
Partials 2 2 ☔ View full report in Codecov by Sentry. |
github-actions
bot
removed
the
github_actions:pull-request
|
Lighthouse Results
|
There was a problem hiding this comment.
Pull request overview
This PR adjusts how FunctionSignature renders return types so that top-level kind: 'return' entries are displayed outside the titled signature container, aligning the UI with the Slack discussion and provided validation screenshot.
Changes:
- Split
itemsinto non-return “attributes” vs.kind: 'return'entries and render them separately when atitleis provided. - Extend the Storybook
Nestedstory to include an additional top-level return example. - Add bottom spacing to the
SignatureRootcontainer and bump@node-core/ui-componentsversion to1.6.3.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| packages/ui-components/src/Containers/FunctionSignature/index.tsx | Separates top-level return items from attribute items when rendering a titled signature. |
| packages/ui-components/src/Containers/FunctionSignature/index.stories.tsx | Adds a nested story case with a top-level return entry to validate the new layout. |
| packages/ui-components/src/Common/Signature/SignatureRoot/index.module.css | Adds mb-1 to the signature root container to improve spacing between sections. |
| packages/ui-components/package.json | Bumps the package version from 1.6.2 to 1.6.3. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
packages/ui-components/src/Containers/FunctionSignature/index.tsx
Outdated
Show resolved
Hide resolved
|
Really good stuff <3 |
|
|
||
| const FunctionSignature: FC<FunctionSignatureProps> = ({ title, items }) => { | ||
| if (title) { | ||
| const attributes: Array<SignatureDefinition> = []; |
There was a problem hiding this comment.
I would:
const { attributes, returnTypes } = items.reduce(
(acc, item) => {
(item.kind === 'return' ? acc.returnTypes : acc.attributes).push(item);
return acc;
},
{ attributes: [] as SignatureDefinition[], returnTypes: [] as SignatureDefinition[] }
);There was a problem hiding this comment.
Or at least
const attributes: SignatureDefinition[] = [];
const returnTypes: SignatureDefinition[] = [];
for (const item of items) {
const target = item.kind === 'return' ? returnTypes : attributes;
target.push(item);
}
Description
As discussed on Slack, this PR includes moving the first return in the
FunctionSignaturecomponent outside of the container.Validation